Add L7 Antrea native NetworkPolicy datapath support #4410

hongliangl · 2022-11-23T17:31:08Z

This PR depends on #4380 #4406

Signed-off-by: Hongliang Liu [email protected]

codecov · 2022-11-23T18:05:11Z

Codecov Report

Merging #4410 (4771389) into main (721fc9e) will decrease coverage by 2.46%.
The diff coverage is 75.78%.

❗ Current head 4771389 differs from pull request most recent head dc97f1f. Consider uploading reports for the commit dc97f1f to get more accurate results

@@            Coverage Diff             @@
##             main    #4410      +/-   ##
==========================================
- Coverage   67.91%   65.44%   -2.47%     
==========================================
  Files         403      402       -1     
  Lines       57698    58411     +713     
==========================================
- Hits        39187    38229     -958     
- Misses      15788    17435    +1647     
- Partials     2723     2747      +24

Flag	Coverage Δ		*Carryforward flag
integration-tests	`34.74% <56.94%> (+0.10%)`	⬆️
kind-e2e-tests	`46.38% <52.87%> (-1.27%)`	⬇️	Carriedforward from 22489ad
unit-tests	`51.04% <51.11%> (-5.67%)`	⬇️	Carriedforward from 22489ad

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
cmd/antrea-agent/agent.go	`0.00% <0.00%> (ø)`
pkg/agent/agent_linux.go	`4.65% <ø> (ø)`
pkg/agent/agent_windows.go	`1.20% <ø> (ø)`
pkg/agent/config/node_config.go	`76.19% <ø> (-19.81%)`	⬇️
pkg/agent/controller/networkpolicy/allocator.go	`83.18% <ø> (ø)`
pkg/agent/types/networkpolicy.go	`65.45% <ø> (-24.29%)`	⬇️
pkg/agent/controller/networkpolicy/reconciler.go	`70.27% <60.00%> (-0.31%)`	⬇️
pkg/agent/openflow/pipeline.go	`77.96% <61.53%> (-13.11%)`	⬇️
...ntroller/networkpolicy/networkpolicy_controller.go	`73.34% <63.63%> (ø)`
pkg/ovs/openflow/ofctrl_action.go	`80.91% <71.42%> (-7.64%)`	⬇️
... and 78 more

hongliangl · 2022-12-01T14:07:28Z

Could you help review this PR? @tnqn @qiyueyao I have polished the code and I'll add e2e tests in this PR later.

pkg/agent/agent_linux.go

pkg/agent/config/node_config.go

pkg/agent/controller/l7networkpolicy/suricata/reconciler.go

pkg/agent/controller/networkpolicy/cache.go

pkg/agent/controller/networkpolicy/networkpolicy_controller.go

pkg/agent/openflow/fields.go

pkg/controller/networkpolicy/networkpolicy_controller.go

pkg/agent/agent_linux.go

pkg/agent/agent.go

pkg/agent/config/node_config.go

pkg/agent/controller/l7networkpolicy/suricata/reconciler.go

pkg/agent/controller/networkpolicy/cache.go

pkg/agent/controller/networkpolicy/l7engine/reconciler.go

pkg/controller/networkpolicy/networkpolicy_controller.go

pkg/agent/openflow/network_policy.go

pkg/agent/controller/networkpolicy/l7engine/reconciler.go

pkg/agent/config/node_config.go

tnqn

not finished, publishing comments so far

pkg/agent/agent_linux.go

pkg/agent/config/node_config.go

pkg/controller/networkpolicy/networkpolicy_controller.go

pkg/agent/controller/networkpolicy/allocator_test.go

hongliangl · 2022-12-20T16:00:49Z

pkg/ovs/openflow/ofctrl_builder.go

+	// - to match VLAN ID 0 only, the mask should be 0x1fff (openflow15.OFPVID_PRESENT | protocol.VID_MASK)
+	// - to match all VLAN IDs, the mask should be 0x1000 (openflow15.OFPVID_PRESENT)
+	// When VLAN ID is not 0, the mask can be nil or 0x1fff, and patch it to 0x1fff
+	if !nonVLAN && vlanID != 0 {


vlanID != 0 && vlanMaks == nil {
}

hongliangl · 2022-12-20T16:02:59Z

pkg/ovs/openflow/ofctrl_builder.go

@@ -42,8 +43,27 @@ func (b *ofFlowBuilder) MatchTunMetadata(index int, data uint32) FlowBuilder {

 // MatchVLAN matches the VLAN VID. It holds the VLAN VID in its least significant 12 bits.


Add some comments about how to use this interface.

Match a specific VLAN
Match all VLANs
Not match any VLAN

hongliangl · 2022-12-20T16:04:18Z

pkg/ovs/openflow/ofctrl_builder.go

+		mask = *vlanMask
+	}
+	var matchStr string
+	if (mask & defaultVLANMask) == defaultVLANMask {


Suggested change

if (mask & defaultVLANMask) == defaultVLANMask {

if mask == defaultVLANMask {

hongliangl · 2022-12-20T16:08:41Z

pkg/ovs/openflow/testing/utils.go

+	if (mask & (protocol.VID_MASK | openflow15.OFPVID_PRESENT)) == (protocol.VID_MASK | openflow15.OFPVID_PRESENT) {
+		return fmt.Sprintf("dl_vlan=%d", value&protocol.VID_MASK)
+	}
+	return fmt.Sprintf("vlan_tci=0x%04x/0x%04x", value&openflow15.OFPVID_PRESENT, mask&openflow15.OFPVID_PRESENT)


Suggested change

return fmt.Sprintf("vlan_tci=0x%04x/0x%04x", value&openflow15.OFPVID_PRESENT, mask&openflow15.OFPVID_PRESENT)

return fmt.Sprintf("vlan_tci=0x%04x/0x%04x", value&openflow15.OFPVID_PRESENT, openflow15.OFPVID_PRESENT)

hongliangl · 2022-12-20T16:16:16Z

test/e2e/l7networkpolicy_test.go

+	}
+	require.NoError(t, data.podWaitForRunning(defaultTimeout, clientPodName, data.testNamespace))
+
+	serverPodName := "test-l7-http-server"


Add some comments about the priority of Policies

hongliangl · 2022-12-20T16:19:08Z

test/e2e/l7networkpolicy_test.go

+			assert.NotNil(t, err)
+
+			// Non-HTTP connections should be rejected by Suricata.
+			if ip.To4() != nil {


Why IPv6 not drop?

pkg/agent/controller/networkpolicy/l7engine/reconciler.go

pkg/agent/controller/networkpolicy/l7engine/reconciler_test.go

pkg/ovs/openflow/ofctrl_builder.go

test/e2e/l7networkpolicy_test.go

tnqn

LGTM

tnqn

I found two problems when testing it:

suricata became unresponsive after running a while, no traffic is forwarded, no suricatasc command is responding; this may be related to how suricata instance is started
restarting agent doesn't re-install existing rules, which should be handled in syncRules

Signed-off-by: Hongliang Liu <[email protected]>

tnqn · 2022-12-22T04:31:12Z

/test-all

tnqn · 2022-12-22T05:28:34Z

/test-all

qiyueyao

Looks great, I don't have further comments for now, and I believe the above unclosed conversations have been resolved in the latest commit.

tnqn · 2022-12-22T09:53:53Z

/skip-all which have succeeded

hongliangl · 2023-01-19T06:37:25Z

pkg/agent/agent_linux.go

+
+// prepareL7NetworkPolicyInterfaces creates two OVS internal ports. An application-aware engine will connect to OVS
+// through these two ports.
+func (i *Initializer) prepareL7NetworkPolicyInterfaces() error {


Signed-off-by: Hongliang Liu <[email protected]>

hongliangl requested a review from tnqn November 23, 2022 17:33

hongliangl force-pushed the 20221109-l7-policy-dp branch from 1bb79b6 to c0318b1 Compare November 23, 2022 17:44

hongliangl force-pushed the 20221109-l7-policy-dp branch 3 times, most recently from 8dee680 to b04d745 Compare December 1, 2022 14:06

hongliangl requested a review from qiyueyao December 1, 2022 14:06

hongliangl force-pushed the 20221109-l7-policy-dp branch from b04d745 to 19d54ce Compare December 1, 2022 15:19

vicky-liu added this to the Antrea v1.10 release milestone Dec 2, 2022

hongliangl force-pushed the 20221109-l7-policy-dp branch 5 times, most recently from 71b4a77 to f126a6b Compare December 5, 2022 13:46

tnqn reviewed Dec 5, 2022

View reviewed changes

qiyueyao reviewed Dec 6, 2022

View reviewed changes

hongliangl force-pushed the 20221109-l7-policy-dp branch 4 times, most recently from a821312 to 28c1399 Compare December 6, 2022 17:21

jianjuns reviewed Dec 6, 2022

View reviewed changes

hongliangl force-pushed the 20221109-l7-policy-dp branch 5 times, most recently from 5cf3651 to 8dddc0e Compare December 13, 2022 10:52

tnqn reviewed Dec 13, 2022

View reviewed changes

hongliangl force-pushed the 20221109-l7-policy-dp branch from 8dddc0e to 22489ad Compare December 14, 2022 06:36

tnqn mentioned this pull request Dec 14, 2022

Add suricata to antrea images #4484

Merged

hongliangl commented Dec 20, 2022

View reviewed changes

hongliangl force-pushed the 20221109-l7-policy-dp branch 4 times, most recently from 7bbfd6f to c8ac822 Compare December 21, 2022 06:18

tnqn reviewed Dec 21, 2022

View reviewed changes

hongliangl mentioned this pull request Dec 21, 2022

Correct unit tests expectations about MatchVLAN in pkg/ovs/openflow #4490

Closed

hongliangl force-pushed the 20221109-l7-policy-dp branch from c8ac822 to 449e8cc Compare December 21, 2022 09:11

hongliangl requested a review from tnqn December 21, 2022 09:20

tnqn previously approved these changes Dec 21, 2022

View reviewed changes

tnqn reviewed Dec 21, 2022

View reviewed changes

hongliangl dismissed tnqn’s stale review via 4b644f5 December 21, 2022 13:53

hongliangl force-pushed the 20221109-l7-policy-dp branch 3 times, most recently from 134bbb3 to a29076a Compare December 21, 2022 15:17

Add L7 Antrea native NetworkPolicy datapath support

dc97f1f

Signed-off-by: Hongliang Liu <[email protected]>

hongliangl force-pushed the 20221109-l7-policy-dp branch from a29076a to dc97f1f Compare December 21, 2022 15:43

tnqn approved these changes Dec 22, 2022

View reviewed changes

qiyueyao approved these changes Dec 22, 2022

View reviewed changes

tnqn merged commit 7a0b581 into antrea-io:main Dec 22, 2022

hongliangl commented Jan 19, 2023

View reviewed changes

GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Jan 27, 2023

Add L7 Antrea native NetworkPolicy datapath support (antrea-io#4410)

59d7f13

Signed-off-by: Hongliang Liu <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add L7 Antrea native NetworkPolicy datapath support #4410

Add L7 Antrea native NetworkPolicy datapath support #4410

hongliangl commented Nov 23, 2022 •

edited

Loading

codecov bot commented Nov 23, 2022 •

edited

Loading

hongliangl commented Dec 1, 2022

tnqn left a comment

hongliangl Dec 20, 2022

hongliangl Dec 20, 2022

hongliangl Dec 20, 2022

hongliangl Dec 20, 2022

hongliangl Dec 20, 2022

hongliangl Dec 20, 2022

hongliangl Dec 20, 2022

tnqn left a comment

tnqn left a comment

tnqn commented Dec 22, 2022

tnqn commented Dec 22, 2022

qiyueyao left a comment

tnqn commented Dec 22, 2022

hongliangl Jan 19, 2023

		@@ -42,8 +43,27 @@ func (b *ofFlowBuilder) MatchTunMetadata(index int, data uint32) FlowBuilder {

		// MatchVLAN matches the VLAN VID. It holds the VLAN VID in its least significant 12 bits.

	if (mask & defaultVLANMask) == defaultVLANMask {
	if mask == defaultVLANMask {

	return fmt.Sprintf("vlan_tci=0x%04x/0x%04x", value&openflow15.OFPVID_PRESENT, mask&openflow15.OFPVID_PRESENT)
	return fmt.Sprintf("vlan_tci=0x%04x/0x%04x", value&openflow15.OFPVID_PRESENT, openflow15.OFPVID_PRESENT)

Add L7 Antrea native NetworkPolicy datapath support #4410

Add L7 Antrea native NetworkPolicy datapath support #4410

Conversation

hongliangl commented Nov 23, 2022 • edited Loading

codecov bot commented Nov 23, 2022 • edited Loading

Codecov Report

hongliangl commented Dec 1, 2022

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Dec 22, 2022

tnqn commented Dec 22, 2022

qiyueyao left a comment

Choose a reason for hiding this comment

tnqn commented Dec 22, 2022

Choose a reason for hiding this comment

hongliangl commented Nov 23, 2022 •

edited

Loading

codecov bot commented Nov 23, 2022 •

edited

Loading